Fix multiple MIDI out issues #24944
Fix multiple MIDI out issues #24944
Conversation
FluidSequencer: store 16 bit velocity in midi 2.0 note messages. FluidSynth: explicitely use 7 bit velocity, 14 bit pitch bend and 7bit CC data, automatically scaled down if needed. midi::Event: Fix midi 2 to 1 conversion details. macOS Core Midi: Use correct packet length. Improving MIDI output in FluidSynth / FluidSequencer: Cleanup hanging notes: Send 'all notes off' and related control reset messages to midi out port. Midi out channel: Use staff index as midi out channel. Addresses issue musescore#18382 in a simple way.
|
Wouldn't similar changes as those you did for Mac be needed for Windows and Linux too? |
|
For Windows the MIDI call is midiOutShortMsg, taking a 32 bit Word containing a Midi 1.0 message. It requires no packet length. |
| @@ -187,6 +187,7 @@ void CoreMidiInPort::initCore() | |||
| } | |||
|
|
|||
| QString portName = "MuseScore MIDI input port"; | |||
| static bool doLog = false; // kors::logger::Logger::instance()->isLevel(kors::logger::Level::Debug); | |||
There was a problem hiding this comment.
See audiobuffer.cpp for the preferred way for such things; for example at the top of the file
//#define DEBUG_COREMIDIINPORT
#ifdef DEBUG_COREMIDIINPORT
#define LOG_MIDI LOGD
#else
#define LOG_MIDI LOGN
#endifAnd then just write
LOG_MIDI() << "Receiving MIDIEventPacket with " << packet->wordCount << " words";|
|
||
| if (__builtin_available(macOS 11.0, *)) { | ||
| MIDIProtocolID protocolId = configuration()->useMIDI20Output() ? m_core->destinationProtocolId : kMIDIProtocol_1_0; | ||
| MIDIProtocolID protocolId = kMIDIProtocol_2_0; // configuration()->useMIDI20Output() ? m_core->destinationProtocolId : kMIDIProtocol_1_0; |
There was a problem hiding this comment.
What about this change? Looks like that removes the effect of the checkbox added in #12329. If your changes means that we won't need that checkbox anymore, then it would be better to remove it, rather than leaving it there doing nothing.
Also, looks like all the destinationProtocolId stuff becomes unused too.
There was a problem hiding this comment.
Apple says, the Core Midi framework translates between Midi 2.0 and Midi 1.0 messages behind the scenes.
So checking the preferred protocol of the output port is optional and merely allows to filter out ahead what would be thrown away anyway. But I think it is cleaner not doing that.
Regarding the checkbox: Quote from #12329: A use case for this is the following: on macOS, the MIDI IAC Driver reports that it supports MIDI 2.0, but if you connect any MIDI 1.0 apps to that driver, those apps won't understand the MIDI 2.0 messages. In that case, you want to force MuseScore to produce MIDI 1.0 output even though the receiver reports that it supports MIDI 2.0.
If everything works like Apple says, then this should not be a problem. And from my testing my new code works with IAC Driver and Midi 1.0 app connected.
Leaving the checkbox in might be a good idea for some transitioning period for easily testing the two code paths (macOS 11 + midi 2.0 vs old macOS + midi 1.0 packet list) on a recent macOS. Or remove the checkbox but keep the setting option in a less prominent Debug-UI. What do you prefer?
Anyway, I am happy to do the code changes that you suggested, but I would prefer to leave the UI changes to someone who better knows how to do that.
Resolves: #22354
Resolves: #18382
For a long time the midi output was broken in MuseScore 4. Due to inconsistent use of MIDI 2.0 and MIDI 1.0 data values a midi 1.0 note velocity of 64 would be interpreted in Midi 2.0 as very small and substitued by 1. The pull request fixes this by small changes in fluid synth event generation and event interpretation code.
Also handling of Midi event lists on the mac, both output and input was not correct. I fixed it as well. I added some needed utilities and fixed some code in Event type that I noticed was incorrect.
I tested Midi Input and output on the Mac.
Another change is usability of MIDI output.
Currently the user has no way to assign a port and channel to a staff. As a workaround I suggest using the staff index as the channel number (implemented).
When stopping playback, now also All-Note-Off commands are sent to MIDI output.